Enhance/wind farm parameters#15
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands the wind-power sampling interface by extending WindFarmParameters and wiring the new options through to the Python wind_power_timeseries call, with corresponding test updates.
Changes:
- Extend
WindFarmParameterswithturbine_power_curve,sigma, andwakeloss, and changeorientation/shapedefaults tonothingwithshapenow numeric. - Convert a Julia
DataFramepower curve into a PandasSeriesbefore calling the Python sampler. - Update/extend wind-power tests and test utilities to reflect the new parameters and a shorter test horizon.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/datastructures.jl |
Adds new wind parameters, validates them, and passes them through to the Python sampler. |
src/utils.jl |
Adds to_pandas_series helper for converting a power-curve DataFrame to Pandas. |
test/utils.jl |
Updates wind test graph helper to accept/pass new parameters and uses a shorter horizon. |
test/test_windpower.jl |
Updates parameter tests, adds validation tests, and refactors wind mathematical formulation tests. |
test/runtests.jl |
Adds DataFrames import for test usage. |
test/Project.toml |
Adds DataFrames as a test dependency. |
NEWS.md |
Updates release notes describing the wind parameter extensions. |
docs/src/library/internals/methods-EMLI.md |
Adds to_pandas_series to the internal methods list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
It generally looks good. Only a few minor comments. Specifically, I think it is important that we think where we describe the parameters.
| - **`turbine_power_curve::Union{String, DataFrame, Nothing}`** optional power curve input | ||
| (e.g. curve name or dataset-based interpolated curve), default: `nothing`. | ||
| For `String` input, available options are: "VestasV80", "Tradewind_lowland", "Tradewind_upland", | ||
| "Tradewind_offshore", "Tradewind_offshore_2030", "IEA_15MW_240_RWT", "IEA_10MW_198_RWT", | ||
| "NREL_5MW_126_RWT", and "DTU_10MW_178_RWT". | ||
| For `DataFrame` input, the `DataFrame` must contain two columns: "wind_speed" and "power_curve", where | ||
| "wind_speed" is the wind speed in m/s and "power_curve" is the normalized power output (both must be non-negative) | ||
| corresponding to each wind speed (values are normalized by default by the `wind_power_timeseries` module). | ||
| Values are set to zero for wind speeds outside the range of the provided power curve. | ||
| Must have at least 2 rows to allow for interpolation. |
There was a problem hiding this comment.
I would think it is beneficial here to use a note admonition with two separate paragraphs for the String and DataFrame input.
| - **`sigma::Union{Real, Nothing}`** optional Ninja smoothing parameter, default: `nothing`. | ||
| - **`wakeloss::Union{Real, Nothing}`** optional Ninja wakeloss parameter, default: `nothing`. |
There was a problem hiding this comment.
Do I see it correctly, that it is only required when using Ninja? While we have not changed it in this PR, I would say that it would be beneficial to highlight which parameters are required for each method in the documentation or more detailed in the docstring here. That can be set as a warning admonition.
Co-authored-by: Julian Straus <104911227+JulStraus@users.noreply.github.com>
This PR enables more options from sampling the
wind_power_timeseriesrepository. The PR depends on this MR to be merged with main.WindFarmParametersto include an optionalturbine_power_curveargument, which allows users to specify a custom power curve for the wind turbine.The
turbine_power_curveis expected to be aDataFramewith columnswind_speedandpower_curve, wherewind_speedrepresents the wind speed values (in m/s) andpower_curverepresents the corresponding power output of the turbine at those wind speeds.sigmaandwakelosswere also added toWindFarmParametersto enable all options of thewind_power_timeseriesrepository.shapeparameter was corrected from a string to a float.